feat: Add Jobs API infrastructure (PR 1 of 3) #7169
Conversation
dosubot
bot
added
the
size:XL
📝 WalkthroughWalkthroughAdds a ComfyUI Jobs fetcher module that validates API responses, retrieves job history, queue, and individual job details, assigns synthetic priorities, extracts embedded workflows, and exposes these functions via a consolidated public API surface with accompanying types and tests. Changes
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (4)
🧰 Additional context used📓 Path-based instructions (13)**/*.{vue,ts,tsx}📄 CodeRabbit inference engine (.cursorrules)
Files:
**/*.{ts,tsx,js}📄 CodeRabbit inference engine (.cursorrules)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursorrules)
Files:
**/*.{ts,tsx,js,vue}📄 CodeRabbit inference engine (.cursorrules)
Files:
**/*.{ts,tsx,js,jsx,vue}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx,vue}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.ts📄 CodeRabbit inference engine (CLAUDE.md)
Files:
tests-ui/**/*.test.{js,ts,jsx,tsx}📄 CodeRabbit inference engine (tests-ui/CLAUDE.md)
Files:
**/*.{test,spec}.{ts,tsx,js}📄 CodeRabbit inference engine (AGENTS.md)
Files:
src/**/*.{vue,ts}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
src/**/*.ts📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
src/**/*.{ts,tsx,vue}📄 CodeRabbit inference engine (src/CLAUDE.md)
Files:
src/**/*.{vue,ts,tsx}📄 CodeRabbit inference engine (src/CLAUDE.md)
Files:
🧠 Learnings (12)📚 Learning: 2025-11-24T19:48:03.270ZApplied to files:
📚 Learning: 2025-11-24T19:48:03.270ZApplied to files:
📚 Learning: 2025-11-24T19:47:22.909ZApplied to files:
📚 Learning: 2025-11-24T19:48:03.270ZApplied to files:
📚 Learning: 2025-11-24T19:48:03.270ZApplied to files:
📚 Learning: 2025-11-24T19:48:03.270ZApplied to files:
📚 Learning: 2025-11-24T19:48:09.318ZApplied to files:
📚 Learning: 2025-11-24T19:47:22.909ZApplied to files:
📚 Learning: 2025-11-24T19:48:03.270ZApplied to files:
📚 Learning: 2025-11-24T19:47:56.371ZApplied to files:
📚 Learning: 2025-11-24T19:48:23.088ZApplied to files:
📚 Learning: 2025-11-24T19:48:09.318ZApplied to files:
🧬 Code graph analysis (3)tests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.ts (1)
src/platform/remote/comfyui/jobs/types/jobTypes.ts (1)
src/platform/remote/comfyui/jobs/fetchers/fetchJobs.ts (5)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
🔇 Additional comments (20)
Comment |
🎭 Playwright Test Results⏰ Completed at: 12/05/2025, 12:40:52 AM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 12/05/2025, 12:32:06 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.2 MB (baseline 3.2 MB) • ⚪ 0 BMain entry bundles and manifests
Graph Workspace — 974 kB (baseline 974 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Views & Navigation — 6.54 kB (baseline 6.54 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Panels & Settings — 298 kB (baseline 298 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
UI Components — 173 kB (baseline 173 kB) • ⚪ 0 BReusable component library chunks
Data & Services — 12.5 kB (baseline 12.5 kB) • ⚪ 0 BStores, services, APIs, and repositories
Utilities & Hooks — 2.94 kB (baseline 2.94 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Vendor & Third-Party — 8.56 MB (baseline 8.56 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 3.81 MB (baseline 3.81 MB) • ⚪ 0 BBundles that do not match a named category
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
src/platform/remote/comfyui/jobs/fetchers/fetchJobs.ts(1 hunks)src/platform/remote/comfyui/jobs/index.ts(1 hunks)src/platform/remote/comfyui/jobs/types/jobTypes.ts(1 hunks)src/scripts/api.ts(3 hunks)tests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (13)
**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{vue,ts,tsx}: Leverage VueUse functions for performance-enhancing utilities
Use vue-i18n in Composition API for any string literals and place new translation entries in src/locales/en/main.json
Files:
src/scripts/api.tssrc/platform/remote/comfyui/jobs/types/jobTypes.tssrc/platform/remote/comfyui/jobs/fetchers/fetchJobs.tstests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.tssrc/platform/remote/comfyui/jobs/index.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursorrules)
Use es-toolkit for utility functions
Files:
src/scripts/api.tssrc/platform/remote/comfyui/jobs/types/jobTypes.tssrc/platform/remote/comfyui/jobs/fetchers/fetchJobs.tstests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.tssrc/platform/remote/comfyui/jobs/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Use TypeScript for type safety
**/*.{ts,tsx}: Never useanytype - use proper TypeScript types
Never useas anytype assertions - fix the underlying type issue
Files:
src/scripts/api.tssrc/platform/remote/comfyui/jobs/types/jobTypes.tssrc/platform/remote/comfyui/jobs/fetchers/fetchJobs.tstests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.tssrc/platform/remote/comfyui/jobs/index.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (.cursorrules)
Implement proper error handling in components and services
**/*.{ts,tsx,js,vue}: Use 2-space indentation, single quotes, no semicolons, and maintain 80-character line width as configured in.prettierrc
Organize imports by sorting and grouping by plugin, and runpnpm formatbefore committing
Files:
src/scripts/api.tssrc/platform/remote/comfyui/jobs/types/jobTypes.tssrc/platform/remote/comfyui/jobs/fetchers/fetchJobs.tstests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.tssrc/platform/remote/comfyui/jobs/index.ts
src/**/*.{vue,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles
Implement proper error handling
Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
Files:
src/scripts/api.tssrc/platform/remote/comfyui/jobs/types/jobTypes.tssrc/platform/remote/comfyui/jobs/fetchers/fetchJobs.tssrc/platform/remote/comfyui/jobs/index.ts
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
Files:
src/scripts/api.tssrc/platform/remote/comfyui/jobs/types/jobTypes.tssrc/platform/remote/comfyui/jobs/fetchers/fetchJobs.tssrc/platform/remote/comfyui/jobs/index.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use camelCase for variable and setting names in TypeScript/Vue files
Files:
src/scripts/api.tssrc/platform/remote/comfyui/jobs/types/jobTypes.tssrc/platform/remote/comfyui/jobs/fetchers/fetchJobs.tstests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.tssrc/platform/remote/comfyui/jobs/index.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,vue}: Useconst settingStore = useSettingStore()andsettingStore.get('Comfy.SomeSetting')to retrieve settings in TypeScript/Vue files
Useawait settingStore.set('Comfy.SomeSetting', newValue)to update settings in TypeScript/Vue files
Check server capabilities usingapi.serverSupportsFeature('feature_name')before using enhanced features
Useapi.getServerFeature('config_name', defaultValue)to retrieve server feature configurationEnforce ESLint rules for Vue + TypeScript including: no floating promises, no unused imports, and i18n raw text restrictions in templates
Files:
src/scripts/api.tssrc/platform/remote/comfyui/jobs/types/jobTypes.tssrc/platform/remote/comfyui/jobs/fetchers/fetchJobs.tstests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.tssrc/platform/remote/comfyui/jobs/index.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Define dynamic setting defaults using runtime context with functions in settings configuration
UsedefaultsByInstallVersionproperty for gradual feature rollout based on version in settings configuration
Files:
src/scripts/api.tssrc/platform/remote/comfyui/jobs/types/jobTypes.tssrc/platform/remote/comfyui/jobs/fetchers/fetchJobs.tstests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.tssrc/platform/remote/comfyui/jobs/index.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.{ts,tsx,vue}: Sanitize HTML with DOMPurify to prevent XSS attacks
Avoid using @ts-expect-error; use proper TypeScript types instead
Use es-toolkit for utility functions instead of other utility libraries
Implement proper TypeScript types throughout the codebase
Files:
src/scripts/api.tssrc/platform/remote/comfyui/jobs/types/jobTypes.tssrc/platform/remote/comfyui/jobs/fetchers/fetchJobs.tssrc/platform/remote/comfyui/jobs/index.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/scripts/api.tssrc/platform/remote/comfyui/jobs/types/jobTypes.tssrc/platform/remote/comfyui/jobs/fetchers/fetchJobs.tssrc/platform/remote/comfyui/jobs/index.ts
tests-ui/**/*.test.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (tests-ui/CLAUDE.md)
tests-ui/**/*.test.{js,ts,jsx,tsx}: Write tests for new features
Follow existing test patterns in the codebase
Use existing test utilities rather than writing custom utilities
Mock external dependencies in tests
Always prefer vitest mock functions over writing verbose manual mocks
Files:
tests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.ts
**/*.{test,spec}.{ts,tsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
Unit and component tests should be located in
tests-ui/or co-located with components assrc/components/**/*.{test,spec}.ts; E2E tests should be inbrowser_tests/
Files:
tests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.ts
🧠 Learnings (9)
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Write tests for new features
Applied to files:
tests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Mock external dependencies in tests
Applied to files:
tests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Test user workflows in browser tests
Applied to files:
tests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Always prefer vitest mock functions over writing verbose manual mocks
Applied to files:
tests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Follow existing test patterns in the codebase
Applied to files:
tests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Use existing test utilities rather than writing custom utilities
Applied to files:
tests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.ts
📚 Learning: 2025-11-24T19:48:09.318Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursor/rules/unit-test.mdc:0-0
Timestamp: 2025-11-24T19:48:09.318Z
Learning: Applies to test/**/*.{test,spec}.{js,ts,jsx,tsx} : Mocks should be cleanly written and easy to understand, with reusable mocks where possible
Applied to files:
tests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.ts
📚 Learning: 2025-11-24T19:48:23.088Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T19:48:23.088Z
Learning: Use Vitest (with happy-dom) for unit and component tests, and Playwright for E2E tests
Applied to files:
tests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.ts
📚 Learning: 2025-11-24T19:48:09.318Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursor/rules/unit-test.mdc:0-0
Timestamp: 2025-11-24T19:48:09.318Z
Learning: Applies to test/**/*.{test,spec}.{js,ts,jsx,tsx} : Use `vitest` for unit testing in this project
Applied to files:
tests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.ts
🧬 Code graph analysis (2)
src/platform/remote/comfyui/jobs/types/jobTypes.ts (1)
src/schemas/apiSchema.ts (2)
resultItemType(17-17)zTaskOutput(245-245)
tests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.ts (1)
src/platform/remote/comfyui/jobs/fetchers/fetchJobs.ts (4)
fetchHistory(77-90)fetchQueue(96-118)fetchJobDetail(127-144)extractWorkflow(150-160)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: lint-and-format
- GitHub Check: setup
- GitHub Check: collect
- GitHub Check: test
🔇 Additional comments (19)
src/platform/remote/comfyui/jobs/fetchers/fetchJobs.ts (6)
1-19: LGTM!Clean module structure with clear documentation and proper imports. The file is well-organized with section separators.
34-54: LGTM!Good defensive implementation with proper error handling. The function correctly returns safe defaults on both network errors and non-OK responses, and uses Zod for runtime validation.
63-71: LGTM!The nullish coalescing operator correctly preserves server-provided priority while assigning synthetic priorities when not present.
77-90: LGTM!Clean implementation with appropriate priority assignment for history items.
96-118: LGTM!The priority computation correctly ensures the ordering: pending > running > history. The separation of jobs by status is clean and efficient.
127-144: LGTM!Good error handling with appropriate log levels -
warnfor not-found cases anderrorfor exceptions.src/platform/remote/comfyui/jobs/index.ts (1)
1-14: LGTM!Clean barrel file with proper separation of value and type exports. Good public API surface for the module.
tests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.ts (5)
1-26: LGTM!Clean helper functions for creating mock data. The helpers are reusable and make tests readable. Based on learnings, vitest mock functions are correctly used.
45-126: LGTM!Comprehensive test coverage for
fetchHistoryincluding priority assignment, server-provided priority preservation, and error handling scenarios.
128-183: LGTM!Good test coverage including verification that pending jobs have higher priority than running jobs.
185-226: LGTM!Proper test coverage for
fetchJobDetailincluding the undefined return cases.
228-259: LGTM!Good coverage of the
extractWorkflowfunction including edge cases for missing workflow data.src/scripts/api.ts (3)
50-55: LGTM!Clean import structure with clear aliasing to distinguish legacy from new Jobs API functions.
933-947: LGTM!Clean refactoring to use the renamed legacy fetcher while maintaining backward compatibility.
1292-1333: LGTM!Well-implemented new methods that follow the existing patterns in the class. Error handling is consistent with other methods like
getQueue()andgetHistory().src/platform/remote/comfyui/jobs/types/jobTypes.ts (4)
1-11: LGTM!Clear module documentation and proper imports. Good use of existing schemas from
apiSchema.ts.
17-31: LGTM!Good use of
.passthrough()for forward compatibility with future API changes.
37-52: LGTM!Well-structured error schema with appropriate optionality for fields that may not always be present.
54-116: LGTM!Excellent schema design with proper composition and type derivation. The
JobListItemtype correctly extendsRawJobListItemwith a requiredpriorityfield for the synthetic priority system.
| export function extractWorkflow( | ||
| job: JobDetail | undefined | ||
| ): ComfyWorkflowJSON | undefined { | ||
| // Cast is safe - workflow will be validated by loadGraphData -> validateComfyWorkflow | ||
| const workflowData = job?.workflow as | ||
| | { extra_data?: { extra_pnginfo?: { workflow?: unknown } } } | ||
| | undefined | ||
| return workflowData?.extra_data?.extra_pnginfo?.workflow as | ||
| | ComfyWorkflowJSON | ||
| | undefined | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider type guards or Zod for safer workflow extraction.
The type casts bypass TypeScript's type checking. While the comment explains downstream validation will catch issues, a runtime type check here would provide earlier error detection and better type safety.
Consider using a type guard or Zod schema:
+import { zComfyWorkflow } from '@/platform/workflow/validation/schemas/workflowSchema'
+
export function extractWorkflow(
job: JobDetail | undefined
): ComfyWorkflowJSON | undefined {
- // Cast is safe - workflow will be validated by loadGraphData -> validateComfyWorkflow
- const workflowData = job?.workflow as
- | { extra_data?: { extra_pnginfo?: { workflow?: unknown } } }
- | undefined
- return workflowData?.extra_data?.extra_pnginfo?.workflow as
- | ComfyWorkflowJSON
- | undefined
+ const workflowData = job?.workflow
+ if (
+ typeof workflowData === 'object' &&
+ workflowData !== null &&
+ 'extra_data' in workflowData
+ ) {
+ const extraData = (workflowData as Record<string, unknown>).extra_data
+ if (typeof extraData === 'object' && extraData !== null && 'extra_pnginfo' in extraData) {
+ const pnginfo = (extraData as Record<string, unknown>).extra_pnginfo
+ if (typeof pnginfo === 'object' && pnginfo !== null && 'workflow' in pnginfo) {
+ return (pnginfo as Record<string, unknown>).workflow as ComfyWorkflowJSON | undefined
+ }
+ }
+ }
+ return undefined
}Committable suggestion skipped: line range outside the PR's diff.
ric-yu
force-pushed
the
jobs-api-pr1-infrastructure
branch
from
de0d169 to
5cb2579
Compare
Adds Jobs API types, fetchers, and new API methods without breaking existing code. This is the foundation for migrating from legacy /history and /queue endpoints to the unified /jobs endpoint. New files: - src/platform/remote/comfyui/jobs/types/jobTypes.ts - Zod schemas for Jobs API - src/platform/remote/comfyui/jobs/fetchers/fetchJobs.ts - Fetchers for /jobs endpoint - src/platform/remote/comfyui/jobs/index.ts - Barrel exports - tests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.ts API additions (non-breaking): - api.getQueueFromJobsApi() - Queue from /jobs endpoint - api.getHistoryFromJobsApi() - History from /jobs endpoint - api.getJobDetail() - Full job details including workflow and outputs Part of Jobs API migration. See docs/JOBS_API_MIGRATION_PLAN.md for details. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
src/platform/remote/comfyui/jobs/fetchers/fetchJobs.ts(1 hunks)src/platform/remote/comfyui/jobs/index.ts(1 hunks)src/platform/remote/comfyui/jobs/types/jobTypes.ts(1 hunks)tests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (13)
**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{vue,ts,tsx}: Leverage VueUse functions for performance-enhancing utilities
Use vue-i18n in Composition API for any string literals and place new translation entries in src/locales/en/main.json
Files:
src/platform/remote/comfyui/jobs/index.tssrc/platform/remote/comfyui/jobs/types/jobTypes.tstests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.tssrc/platform/remote/comfyui/jobs/fetchers/fetchJobs.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursorrules)
Use es-toolkit for utility functions
Files:
src/platform/remote/comfyui/jobs/index.tssrc/platform/remote/comfyui/jobs/types/jobTypes.tstests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.tssrc/platform/remote/comfyui/jobs/fetchers/fetchJobs.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Use TypeScript for type safety
**/*.{ts,tsx}: Never useanytype - use proper TypeScript types
Never useas anytype assertions - fix the underlying type issue
Files:
src/platform/remote/comfyui/jobs/index.tssrc/platform/remote/comfyui/jobs/types/jobTypes.tstests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.tssrc/platform/remote/comfyui/jobs/fetchers/fetchJobs.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (.cursorrules)
Implement proper error handling in components and services
**/*.{ts,tsx,js,vue}: Use 2-space indentation, single quotes, no semicolons, and maintain 80-character line width as configured in.prettierrc
Organize imports by sorting and grouping by plugin, and runpnpm formatbefore committing
Files:
src/platform/remote/comfyui/jobs/index.tssrc/platform/remote/comfyui/jobs/types/jobTypes.tstests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.tssrc/platform/remote/comfyui/jobs/fetchers/fetchJobs.ts
src/**/*.{vue,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles
Implement proper error handling
Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
Files:
src/platform/remote/comfyui/jobs/index.tssrc/platform/remote/comfyui/jobs/types/jobTypes.tssrc/platform/remote/comfyui/jobs/fetchers/fetchJobs.ts
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
Files:
src/platform/remote/comfyui/jobs/index.tssrc/platform/remote/comfyui/jobs/types/jobTypes.tssrc/platform/remote/comfyui/jobs/fetchers/fetchJobs.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use camelCase for variable and setting names in TypeScript/Vue files
Files:
src/platform/remote/comfyui/jobs/index.tssrc/platform/remote/comfyui/jobs/types/jobTypes.tstests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.tssrc/platform/remote/comfyui/jobs/fetchers/fetchJobs.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,vue}: Useconst settingStore = useSettingStore()andsettingStore.get('Comfy.SomeSetting')to retrieve settings in TypeScript/Vue files
Useawait settingStore.set('Comfy.SomeSetting', newValue)to update settings in TypeScript/Vue files
Check server capabilities usingapi.serverSupportsFeature('feature_name')before using enhanced features
Useapi.getServerFeature('config_name', defaultValue)to retrieve server feature configurationEnforce ESLint rules for Vue + TypeScript including: no floating promises, no unused imports, and i18n raw text restrictions in templates
Files:
src/platform/remote/comfyui/jobs/index.tssrc/platform/remote/comfyui/jobs/types/jobTypes.tstests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.tssrc/platform/remote/comfyui/jobs/fetchers/fetchJobs.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Define dynamic setting defaults using runtime context with functions in settings configuration
UsedefaultsByInstallVersionproperty for gradual feature rollout based on version in settings configuration
Files:
src/platform/remote/comfyui/jobs/index.tssrc/platform/remote/comfyui/jobs/types/jobTypes.tstests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.tssrc/platform/remote/comfyui/jobs/fetchers/fetchJobs.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.{ts,tsx,vue}: Sanitize HTML with DOMPurify to prevent XSS attacks
Avoid using @ts-expect-error; use proper TypeScript types instead
Use es-toolkit for utility functions instead of other utility libraries
Implement proper TypeScript types throughout the codebase
Files:
src/platform/remote/comfyui/jobs/index.tssrc/platform/remote/comfyui/jobs/types/jobTypes.tssrc/platform/remote/comfyui/jobs/fetchers/fetchJobs.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/platform/remote/comfyui/jobs/index.tssrc/platform/remote/comfyui/jobs/types/jobTypes.tssrc/platform/remote/comfyui/jobs/fetchers/fetchJobs.ts
tests-ui/**/*.test.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (tests-ui/CLAUDE.md)
tests-ui/**/*.test.{js,ts,jsx,tsx}: Write tests for new features
Follow existing test patterns in the codebase
Use existing test utilities rather than writing custom utilities
Mock external dependencies in tests
Always prefer vitest mock functions over writing verbose manual mocks
Files:
tests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.ts
**/*.{test,spec}.{ts,tsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
Unit and component tests should be located in
tests-ui/or co-located with components assrc/components/**/*.{test,spec}.ts; E2E tests should be inbrowser_tests/
Files:
tests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.ts
🧠 Learnings (9)
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Write tests for new features
Applied to files:
tests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Mock external dependencies in tests
Applied to files:
tests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Test user workflows in browser tests
Applied to files:
tests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Always prefer vitest mock functions over writing verbose manual mocks
Applied to files:
tests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Follow existing test patterns in the codebase
Applied to files:
tests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Use existing test utilities rather than writing custom utilities
Applied to files:
tests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.ts
📚 Learning: 2025-11-24T19:48:09.318Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursor/rules/unit-test.mdc:0-0
Timestamp: 2025-11-24T19:48:09.318Z
Learning: Applies to test/**/*.{test,spec}.{js,ts,jsx,tsx} : Mocks should be cleanly written and easy to understand, with reusable mocks where possible
Applied to files:
tests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.ts
📚 Learning: 2025-11-24T19:48:23.088Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T19:48:23.088Z
Learning: Use Vitest (with happy-dom) for unit and component tests, and Playwright for E2E tests
Applied to files:
tests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.ts
📚 Learning: 2025-11-24T19:48:09.318Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursor/rules/unit-test.mdc:0-0
Timestamp: 2025-11-24T19:48:09.318Z
Learning: Applies to test/**/*.{test,spec}.{js,ts,jsx,tsx} : Use `vitest` for unit testing in this project
Applied to files:
tests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.ts
🧬 Code graph analysis (2)
src/platform/remote/comfyui/jobs/types/jobTypes.ts (1)
src/schemas/apiSchema.ts (2)
resultItemType(17-17)zTaskOutput(245-245)
tests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.ts (1)
src/platform/remote/comfyui/jobs/fetchers/fetchJobs.ts (4)
fetchHistory(77-90)fetchQueue(96-118)fetchJobDetail(127-144)extractWorkflow(150-160)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: deploy-and-comment
- GitHub Check: test
- GitHub Check: setup
- GitHub Check: collect
- GitHub Check: lint-and-format
🔇 Additional comments (2)
src/platform/remote/comfyui/jobs/index.ts (1)
1-14: Jobs API barrel exports look correct and cohesiveCentralizing
extractWorkflow,fetchHistory,fetchJobDetail,fetchQueue, and the key types behind this index gives a clean, future‑proof surface for callers; no issues from my side.src/platform/remote/comfyui/jobs/types/jobTypes.ts (1)
1-116: Jobs API schemas and derived types look consistent and forward‑compatibleThe schema set (status, preview, execution error, raw item, detail, pagination, list response) together with
.passthrough()where appropriate gives you robust validation while still tolerating backend evolution. The split betweenRawJobListItem(as returned) andJobListItem(with guaranteedpriority) is a clean contract for the fetchers and UI; I don’t see any issues here.
| interface FetchJobsRawResult { | ||
| jobs: RawJobListItem[] | ||
| total: number | ||
| offset: number | ||
| } | ||
|
|
||
| /** | ||
| * Fetches raw jobs from /jobs endpoint | ||
| * @internal | ||
| */ | ||
| async function fetchJobsRaw( | ||
| fetchApi: (url: string) => Promise<Response>, | ||
| statuses: JobStatus[], | ||
| maxItems: number = 200, | ||
| offset: number = 0 | ||
| ): Promise<FetchJobsRawResult> { | ||
| const statusParam = statuses.join(',') | ||
| const url = `/jobs?status=${statusParam}&limit=${maxItems}&offset=${offset}` | ||
| try { | ||
| const res = await fetchApi(url) | ||
| if (!res.ok) { | ||
| console.error(`[Jobs API] Failed to fetch jobs: ${res.status}`) | ||
| return { jobs: [], total: 0, offset: 0 } | ||
| } | ||
| const data = zJobsListResponse.parse(await res.json()) | ||
| return { jobs: data.jobs, total: data.pagination.total, offset } | ||
| } catch (error) { | ||
| console.error('[Jobs API] Error fetching jobs:', error) | ||
| return { jobs: [], total: 0, offset: 0 } | ||
| } | ||
| } | ||
|
|
||
| // Large offset to ensure running/pending jobs sort above history | ||
| const QUEUE_PRIORITY_BASE = 1_000_000 | ||
|
|
||
| /** | ||
| * Assigns synthetic priority to jobs. | ||
| * Only assigns if job doesn't already have a server-provided priority. | ||
| */ | ||
| function assignPriority( | ||
| jobs: RawJobListItem[], | ||
| basePriority: number | ||
| ): JobListItem[] { | ||
| return jobs.map((job, index) => ({ | ||
| ...job, | ||
| priority: job.priority ?? basePriority - index | ||
| })) | ||
| } | ||
|
|
||
| /** | ||
| * Fetches history (completed jobs) | ||
| * Assigns synthetic priority starting from total (lower than queue jobs). | ||
| */ | ||
| export async function fetchHistory( | ||
| fetchApi: (url: string) => Promise<Response>, | ||
| maxItems: number = 200, | ||
| offset: number = 0 | ||
| ): Promise<JobListItem[]> { | ||
| const { jobs, total } = await fetchJobsRaw( | ||
| fetchApi, | ||
| ['completed'], | ||
| maxItems, | ||
| offset | ||
| ) | ||
| // History gets priority based on total count (lower than queue) | ||
| return assignPriority(jobs, total - offset) | ||
| } | ||
|
|
||
| /** | ||
| * Fetches queue (in_progress + pending jobs) | ||
| * Pending jobs get highest priority, then running jobs. | ||
| */ | ||
| export async function fetchQueue( | ||
| fetchApi: (url: string) => Promise<Response> | ||
| ): Promise<{ Running: JobListItem[]; Pending: JobListItem[] }> { | ||
| const { jobs } = await fetchJobsRaw( | ||
| fetchApi, | ||
| ['in_progress', 'pending'], | ||
| 200, | ||
| 0 | ||
| ) | ||
|
|
||
| const running = jobs.filter((j) => j.status === 'in_progress') | ||
| const pending = jobs.filter((j) => j.status === 'pending') | ||
|
|
||
| // Pending gets highest priority, then running | ||
| // Both are above any history job due to QUEUE_PRIORITY_BASE | ||
| return { | ||
| Running: assignPriority(running, QUEUE_PRIORITY_BASE + running.length), | ||
| Pending: assignPriority( | ||
| pending, | ||
| QUEUE_PRIORITY_BASE + running.length + pending.length | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| // ============================================================================ | ||
| // Job Detail Fetcher | ||
| // ============================================================================ | ||
|
|
||
| /** | ||
| * Fetches full job details from /jobs/{job_id} | ||
| */ | ||
| export async function fetchJobDetail( | ||
| fetchApi: (url: string) => Promise<Response>, | ||
| promptId: PromptId | ||
| ): Promise<JobDetail | undefined> { | ||
| try { | ||
| const res = await fetchApi(`/jobs/${promptId}`) | ||
|
|
||
| if (!res.ok) { | ||
| console.warn(`Job not found for prompt ${promptId}`) | ||
| return undefined | ||
| } | ||
|
|
||
| return zJobDetail.parse(await res.json()) | ||
| } catch (error) { | ||
| console.error(`Failed to fetch job detail for prompt ${promptId}:`, error) | ||
| return undefined | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Extracts workflow from job detail response. | ||
| * The workflow is nested at: workflow.extra_data.extra_pnginfo.workflow | ||
| */ | ||
| export function extractWorkflow( | ||
| job: JobDetail | undefined | ||
| ): ComfyWorkflowJSON | undefined { | ||
| // Cast is safe - workflow will be validated by loadGraphData -> validateComfyWorkflow | ||
| const workflowData = job?.workflow as | ||
| | { extra_data?: { extra_pnginfo?: { workflow?: unknown } } } | ||
| | undefined | ||
| return workflowData?.extra_data?.extra_pnginfo?.workflow as | ||
| | ComfyWorkflowJSON | ||
| | undefined | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Fetchers, priority logic, and error handling look solid with a couple of trade‑offs to be aware of
Overall the implementation matches the design goals well:
fetchJobsRawcentralizes URL building and response validation, and safely degrades to empty results on any HTTP/parse error.assignPriority’s “only synthesize whenpriorityis nullish” behavior cleanly preserves server ordering while still giving you a total/offset‑based fallback.fetchHistoryandfetchQueuecooperate to ensure the intended ordering (Pending > Running > History), and thetotal - offsetbase in history should keep priorities consistent across pages.fetchJobDetailandextractWorkflowgive callers a simpleundefinedcontract on failure while logging enough context for debugging.
Two non‑blocking considerations:
QUEUE_PRIORITY_BASE = 1_000_000assumes the maximum history priority will remain below that; if very large installations could exceed this, you might eventually want to derive the base from an upper bound (e.g., a config orNumber.MAX_SAFE_INTEGERmargin).- All failures in
fetchJobsRawsurface to callers as “no jobs” rather than a hard error; if future UX needs to distinguish “empty history/queue” from “failed to load”, you may want a variant that returns an error flag alongside the jobs.
Nothing here looks blocking; the current behavior is coherent and in line with the PR scope.
🤖 Prompt for AI Agents
In src/platform/remote/comfyui/jobs/fetchers/fetchJobs.ts around lines 24-160,
address the two reviewer suggestions: (1) Replace the hardcoded
QUEUE_PRIORITY_BASE with a configurable or derived value — e.g., read from
config or compute from a safe upper bound (Number.MAX_SAFE_INTEGER minus a
margin) and use that variable where QUEUE_PRIORITY_BASE is currently referenced;
(2) Make fetchJobsRaw distinguish between “empty result” and “fetch/parse error”
by returning an additional error flag or an Error object (or by throwing) so
callers can opt into treating failures differently from empty lists; update
callers (fetchHistory/fetchQueue) to handle the new return shape or catch the
thrown error accordingly.
| import { describe, expect, it, vi } from 'vitest' | ||
|
|
||
| import { | ||
| extractWorkflow, | ||
| fetchHistory, | ||
| fetchJobDetail, | ||
| fetchQueue | ||
| } from '@/platform/remote/comfyui/jobs' | ||
|
|
||
| // Helper to create a mock job | ||
| function createMockJob( | ||
| id: string, | ||
| status: 'pending' | 'in_progress' | 'completed' = 'completed', | ||
| overrides: Record<string, unknown> = {} | ||
| ) { | ||
| return { | ||
| id, | ||
| status, | ||
| create_time: Date.now(), | ||
| execution_start_time: null, | ||
| execution_end_time: null, | ||
| preview_output: null, | ||
| outputs_count: 0, | ||
| ...overrides | ||
| } | ||
| } | ||
|
|
||
| // Helper to create mock API response | ||
| function createMockResponse( | ||
| jobs: ReturnType<typeof createMockJob>[], | ||
| total: number = jobs.length | ||
| ) { | ||
| return { | ||
| jobs, | ||
| pagination: { | ||
| offset: 0, | ||
| limit: 200, | ||
| total, | ||
| has_more: false | ||
| } | ||
| } | ||
| } | ||
|
|
||
| describe('fetchJobs', () => { | ||
| describe('fetchHistory', () => { | ||
| it('fetches completed jobs', async () => { | ||
| const mockFetch = vi.fn().mockResolvedValue({ | ||
| ok: true, | ||
| json: () => | ||
| Promise.resolve( | ||
| createMockResponse([ | ||
| createMockJob('job1', 'completed'), | ||
| createMockJob('job2', 'completed') | ||
| ]) | ||
| ) | ||
| }) | ||
|
|
||
| const result = await fetchHistory(mockFetch) | ||
|
|
||
| expect(mockFetch).toHaveBeenCalledWith( | ||
| '/jobs?status=completed&limit=200&offset=0' | ||
| ) | ||
| expect(result).toHaveLength(2) | ||
| expect(result[0].id).toBe('job1') | ||
| expect(result[1].id).toBe('job2') | ||
| }) | ||
|
|
||
| it('assigns synthetic priorities', async () => { | ||
| const mockFetch = vi.fn().mockResolvedValue({ | ||
| ok: true, | ||
| json: () => | ||
| Promise.resolve( | ||
| createMockResponse( | ||
| [ | ||
| createMockJob('job1', 'completed'), | ||
| createMockJob('job2', 'completed'), | ||
| createMockJob('job3', 'completed') | ||
| ], | ||
| 3 | ||
| ) | ||
| ) | ||
| }) | ||
|
|
||
| const result = await fetchHistory(mockFetch) | ||
|
|
||
| // Priority should be assigned from total down | ||
| expect(result[0].priority).toBe(3) // total - 0 - 0 | ||
| expect(result[1].priority).toBe(2) // total - 0 - 1 | ||
| expect(result[2].priority).toBe(1) // total - 0 - 2 | ||
| }) | ||
|
|
||
| it('preserves server-provided priority', async () => { | ||
| const mockFetch = vi.fn().mockResolvedValue({ | ||
| ok: true, | ||
| json: () => | ||
| Promise.resolve( | ||
| createMockResponse([ | ||
| createMockJob('job1', 'completed', { priority: 999 }) | ||
| ]) | ||
| ) | ||
| }) | ||
|
|
||
| const result = await fetchHistory(mockFetch) | ||
|
|
||
| expect(result[0].priority).toBe(999) | ||
| }) | ||
|
|
||
| it('returns empty array on error', async () => { | ||
| const mockFetch = vi.fn().mockRejectedValue(new Error('Network error')) | ||
|
|
||
| const result = await fetchHistory(mockFetch) | ||
|
|
||
| expect(result).toEqual([]) | ||
| }) | ||
|
|
||
| it('returns empty array on non-ok response', async () => { | ||
| const mockFetch = vi.fn().mockResolvedValue({ | ||
| ok: false, | ||
| status: 500 | ||
| }) | ||
|
|
||
| const result = await fetchHistory(mockFetch) | ||
|
|
||
| expect(result).toEqual([]) | ||
| }) | ||
| }) | ||
|
|
||
| describe('fetchQueue', () => { | ||
| it('fetches running and pending jobs', async () => { | ||
| const mockFetch = vi.fn().mockResolvedValue({ | ||
| ok: true, | ||
| json: () => | ||
| Promise.resolve( | ||
| createMockResponse([ | ||
| createMockJob('running1', 'in_progress'), | ||
| createMockJob('pending1', 'pending'), | ||
| createMockJob('pending2', 'pending') | ||
| ]) | ||
| ) | ||
| }) | ||
|
|
||
| const result = await fetchQueue(mockFetch) | ||
|
|
||
| expect(mockFetch).toHaveBeenCalledWith( | ||
| '/jobs?status=in_progress,pending&limit=200&offset=0' | ||
| ) | ||
| expect(result.Running).toHaveLength(1) | ||
| expect(result.Pending).toHaveLength(2) | ||
| expect(result.Running[0].id).toBe('running1') | ||
| expect(result.Pending[0].id).toBe('pending1') | ||
| }) | ||
|
|
||
| it('assigns queue priorities above history', async () => { | ||
| const mockFetch = vi.fn().mockResolvedValue({ | ||
| ok: true, | ||
| json: () => | ||
| Promise.resolve( | ||
| createMockResponse([ | ||
| createMockJob('running1', 'in_progress'), | ||
| createMockJob('pending1', 'pending') | ||
| ]) | ||
| ) | ||
| }) | ||
|
|
||
| const result = await fetchQueue(mockFetch) | ||
|
|
||
| // Queue priorities should be above 1_000_000 (QUEUE_PRIORITY_BASE) | ||
| expect(result.Running[0].priority).toBeGreaterThan(1_000_000) | ||
| expect(result.Pending[0].priority).toBeGreaterThan(1_000_000) | ||
| // Pending should have higher priority than running | ||
| expect(result.Pending[0].priority).toBeGreaterThan( | ||
| result.Running[0].priority | ||
| ) | ||
| }) | ||
|
|
||
| it('returns empty arrays on error', async () => { | ||
| const mockFetch = vi.fn().mockRejectedValue(new Error('Network error')) | ||
|
|
||
| const result = await fetchQueue(mockFetch) | ||
|
|
||
| expect(result).toEqual({ Running: [], Pending: [] }) | ||
| }) | ||
| }) | ||
|
|
||
| describe('fetchJobDetail', () => { | ||
| it('fetches job detail by id', async () => { | ||
| const jobDetail = { | ||
| ...createMockJob('job1', 'completed'), | ||
| workflow: { extra_data: { extra_pnginfo: { workflow: {} } } }, | ||
| outputs: { | ||
| '1': { | ||
| images: [{ filename: 'test.png', subfolder: '', type: 'output' }] | ||
| } | ||
| } | ||
| } | ||
| const mockFetch = vi.fn().mockResolvedValue({ | ||
| ok: true, | ||
| json: () => Promise.resolve(jobDetail) | ||
| }) | ||
|
|
||
| const result = await fetchJobDetail(mockFetch, 'job1') | ||
|
|
||
| expect(mockFetch).toHaveBeenCalledWith('/jobs/job1') | ||
| expect(result?.id).toBe('job1') | ||
| expect(result?.outputs).toBeDefined() | ||
| }) | ||
|
|
||
| it('returns undefined for non-ok response', async () => { | ||
| const mockFetch = vi.fn().mockResolvedValue({ | ||
| ok: false, | ||
| status: 404 | ||
| }) | ||
|
|
||
| const result = await fetchJobDetail(mockFetch, 'nonexistent') | ||
|
|
||
| expect(result).toBeUndefined() | ||
| }) | ||
|
|
||
| it('returns undefined on error', async () => { | ||
| const mockFetch = vi.fn().mockRejectedValue(new Error('Network error')) | ||
|
|
||
| const result = await fetchJobDetail(mockFetch, 'job1') | ||
|
|
||
| expect(result).toBeUndefined() | ||
| }) | ||
| }) | ||
|
|
||
| describe('extractWorkflow', () => { | ||
| it('extracts workflow from nested structure', () => { | ||
| const jobDetail = { | ||
| ...createMockJob('job1', 'completed'), | ||
| workflow: { | ||
| extra_data: { | ||
| extra_pnginfo: { | ||
| workflow: { nodes: [], links: [] } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| const workflow = extractWorkflow(jobDetail) | ||
|
|
||
| expect(workflow).toEqual({ nodes: [], links: [] }) | ||
| }) | ||
|
|
||
| it('returns undefined if workflow not present', () => { | ||
| const jobDetail = createMockJob('job1', 'completed') | ||
|
|
||
| const workflow = extractWorkflow(jobDetail) | ||
|
|
||
| expect(workflow).toBeUndefined() | ||
| }) | ||
|
|
||
| it('returns undefined for undefined input', () => { | ||
| const workflow = extractWorkflow(undefined) | ||
|
|
||
| expect(workflow).toBeUndefined() | ||
| }) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Strong coverage for fetchers; consider a couple of small edge‑case tests
These tests do a good job exercising URL construction, synthetic priority behavior, error handling, and extractWorkflow. Two non‑blocking enhancements you might consider:
- Add a
fetchHistorytest with a non‑zerooffsetto lock in thetotal - offsetpriority base across pages. - Mirror the history “non‑ok response” case for
fetchQueue(currently only network rejection is covered) and, optionally, a queue case where the server provides its ownpriorityto document that we intentionally preserve it.
Based on learnings, this aligns well with the Vitest and mocking guidance for tests-ui/ but these extra cases would future‑proof the behavior a bit more.
🤖 Prompt for AI Agents
tests-ui/tests/platform/remote/comfyui/jobs/fetchJobs.test.ts lines 1-260: add
two small tests—one for fetchHistory with a non‑zero offset to verify synthetic
priority calculation uses total - offset correctly across pages, and one for
fetchQueue to handle a non‑ok server response (e.g., ok: false, status: 500)
returning { Running: [], Pending: [] }; optionally also add a queue test where
the server returns a job with an explicit priority to assert that priority is
preserved. Implement these by mocking fetch to return the appropriate response
shape, calling the respective function, and asserting on priorities and
empty-result behavior as described.
5cb2579 to
0a619ec
Compare
Summary
Add Jobs API infrastructure in preparation for migrating from legacy
/history,/history_v2, and/queueendpoints to the unified/jobsAPI.This is PR 1 of 3 - Additive changes only, no breaking changes.
Changes
What:
JobListItem,JobDetail)fetchQueue,fetchHistory,fetchJobDetailfetchers for/jobsendpointextractWorkflowutility for extracting workflow from nested job detail responseNon-breaking: All changes are additive - existing code continues to work
Review Focus
.passthrough()to allow extra API fields - ensures forward compatibility but less strict validationFiles Added
src/platform/remote/comfyui/jobs/- New Jobs API moduletypes/jobTypes.ts- Zod schemas and TypeScript typesfetchers/fetchJobs.ts- API fetchers with validationindex.ts- Barrel exportstests-ui/tests/platform/remote/comfyui/jobs/fetchers/fetchJobs.test.ts- TestsNext PRs
getQueue()andgetHistory()to use Jobs API┆Issue is synchronized with this Notion page by Unito